Skip to content

HIVE-29397: Apply masking to TestBeeLineDriver qtest goldens when overwrite enabled#6609

Open
cyanzheng2926 wants to merge 1 commit into
apache:masterfrom
cyanzheng2926:HIVE-29397_TestBeelineDriver_masking
Open

HIVE-29397: Apply masking to TestBeeLineDriver qtest goldens when overwrite enabled#6609
cyanzheng2926 wants to merge 1 commit into
apache:masterfrom
cyanzheng2926:HIVE-29397_TestBeelineDriver_masking

Conversation

@cyanzheng2926

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Moved qOutProcessor.maskPatterns() under CoreBeeLineDriver.runTest() outside of the if (!overwrite) path so it runs
before both the compare and overwrite paths, matching QTestUtil.checkCliDriverResults().

Why are the changes needed?

Previously masking only run when test.output.overwrite=false. With-Dtest.output.overwrite=true, unmasked output was generated to .q.out, causing normal test runs to fail after masking was applied.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Reproduced and tested as in the Jira description for HIVE-29397 with desc_table_formatted.q:

…rwrite enabled

Mask output before overwriteResults in CoreBeeLineDriver, matching
QTestUtil.checkCliDriverResults().
@ayushtkn

Copy link
Copy Markdown
Member

That ticket seems already assigned to @thomasrebele We shouldn't be picking up tickets like that, unless we confirm the assignee isn't planning to work on it

@cyanzheng2926

cyanzheng2926 commented Jul 16, 2026

Copy link
Copy Markdown
Contributor Author

That ticket seems already assigned to @thomasrebele We shouldn't be picking up tickets like that, unless we confirm the assignee isn't planning to work on it

Hi @ayushtkn thanks for flagging this. Yesterday I was checking with @thomasrebele for the open Hive Jira's to be worked on, he suggested HIVE-29397(along with a few others) as a good candidate for me to try. Happy to double-check with him or adjust if needed. Thanks!

@ayushtkn

Copy link
Copy Markdown
Member

If you have confirmed with him, that fair enough

@thomasrebele

Copy link
Copy Markdown
Contributor

I confirm that I have no current plans to work on HIVE-29397. I thought that I'll look into it when I had created the ticket, but then I got busy with other tickets. Sorry for the inconvenience.

Thank you, @cyanzheng2926, for picking it up!

@cyanzheng2926

Copy link
Copy Markdown
Contributor Author

The failed test (TestReplicationScenariosAcidTables#testSizeOfDatabaseReplicationViaDistCp) looks unrelated to this PR and could be flaky. @ayushtkn can you please help to re-trigger the pre-commit when you have a moment? Thanks!

@sonarqubecloud

Copy link
Copy Markdown

@thomasrebele thomasrebele left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR, @cyanzheng2926! I've checked it with the test case from the description of HIVE-29397. I confirm it works as expected.

System.err.println(">>> FILTERED " + qFile.getName() + ": " + (filterEndTime - queryEndTime)
+ "ms");
qOutProcessor.maskPatterns(qFile.getOutputFile().getPath());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's an extraneous linebreak on line 241.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants